Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Submodule Structure for Forks #149

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

Conversation

alex-connolly
Copy link
Contributor

@alex-connolly alex-connolly commented Dec 11, 2023

Currently, the contracts monorepo doesn't contain contracts which have been forked from upstream repositories (e.g. https://github.com/immutable/wallet-contracts and https://github.com/immutable/seaport). This is to enable us to merge with any future upstream changes in the parent repositories. These contracts are stored separately and published to different npm packages e.g. @imtbl/wallet-contracts, most of which are difficult for developers to discover. This introduces a number of challenges for integrating developers, who expect to find these contracts in @imtbl/contracts. Additionally, this makes it more challenging for us to maintain a clear record of the latest versions of all Immutable contracts, and to monitor all contract changes in one place (the general goals of the monorepo)

Goals:

  • Allow integrating developers to reference forked contracts directly, e.g. Seaport can be imported as @imtbl/contracts/seaport/Seaport.sol, Factory can be imported as @imtbl/contracts/passport/Factory.sol.
  • Allow developers to inspect the latest code of the Immutable contracts they are integrating with
  • Allow us to continue to merge with upstream in these forked repositories

Solution:

  • Create a forks directory with git submodules for all forked repositories
  • Create a forks.json which is interpreted by scripts/build.ts (run via yarn build) to define how these forks should be incorporated into the main codebase
  • All smart contracts are copied out of the fork repos and into the primary codebase (along with their dependencies), e.g. forks/seaport/contracts --> contracts/seaport
  • In future, developers will be able to update the git submodule, update forks.json if necessary, then run yarn.build to commit the new structure to git
  • Decision taken to include the copied contracts in git to allow people browsing the repository to discover them easily

Copy link

openzeppelin-code bot commented Dec 11, 2023

Submodule Structure for Forks

Generated at commit: feb982483c34f73fd3def6a73cca3d94e23a8c33

🚨 Report Summary

Severity Level Results
Contracts Critical
High
Medium
Low
Note
Total
1
1
0
14
35
51
Dependencies Critical
High
Medium
Low
Note
Total
0
0
0
0
0
0

For more details view the full report in OpenZeppelin Code Inspector

@drinkcoffee
Copy link
Contributor

@alex-connolly , this PR has interesting ideas. However, this PR is stale. Are you still actively working on this? Should this PR be abandoned?
My suggestion is that the branch stay open, but the PR is closed. In this way, we have the option to come back to this at a later point if we decide that this capability is adequately useful to dedicate effort to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants